Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libboostasio: close connection on timeout #463

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ricardocrl
Copy link

@ricardocrl ricardocrl commented May 31, 2022

The implementation is inspired in the approach used in libev.h. It will now handle timeout from onNegotiate() in the same manner:

  • heartbeat every: timeout / 2
  • close connection after: timeout * 1.5

As opposed to libev.h I opted for 2 separate timer (sending heartbeat + expire connection), for readability and maintainability purposes.

This PR addresses issue: #464

Comment on lines +516 to +533
* @param timeout The suggested timeout from the server
* @return uint16_t The timeout to use
*/
virtual uint16_t onNegotiate(TcpConnection *connection, uint16_t interval) override
virtual uint16_t onNegotiate(TcpConnection *connection, uint16_t timeout) override
{
// skip if no heartbeats are needed
if (interval == 0) return 0;
if (timeout == 0) return 0;

const auto fd = connection->fileno();

auto iter = _watchers.find(fd);
if (iter == _watchers.end()) return 0;

// set the timer
iter->second->set_timer(connection, interval);
iter->second->set_timers(connection, timeout);

// we agree with the interval
return interval;
// we agree with the timeout
return timeout;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This renaming is to achieve consistency with libev.h.

Although "interval" is the most used term in the library, I feel that calling "interval" wouldn't be intuitive to read "heartbeat interval = interval / 2". So I agree more with the "timeout" term chosen in libev.h

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant